Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code review #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

code review #26

wants to merge 2 commits into from

Conversation

ersel
Copy link
Contributor

@ersel ersel commented Nov 14, 2020

No description provided.

have to keep making new code for any updates!!!
.length is what we need to use but I have tried
and not figured out the correct method yet! */
while (index >3) {index = index -4};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is smart, you just need to use the array.length in your while condition

while (index >array.length)

};

const arrayToCSVString = array => {
// your code here
//pass
return array.toString('')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have used .join here

// your code here
// pass
array.push(element);
//array.push([element]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should get rid of commented out code, commented code is dead code! 💀

//pass
/*filter function maybe???
using filter and then giving a function named that function X.
X % 2 should give back all even numbers but I don't quite understand the maths per usual
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maths is simple here

% 2 is saying

divide the number by 2, if the remainder is zero, then you have an even number :)

@@ -1,61 +1,96 @@
const negate = a => {
// your code here
if (a && a){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return !a would have sufficed here

Copy link
Contributor Author

@ersel ersel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some problems with the numbers and strings modules. I hope you can see the problem with those when you look at them now. If not, please ask a mentor/tutor at the next session to help explain why your functions are not generic enough.

};

const truthiness = a => {
// your code here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return Boolean(a) would have sufficed

};

const startsWith = (char, string) => {
// your code here
};
return (char, string.startsWith("a", "aardvark"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return string.startsWith(char)

@@ -1,45 +1,203 @@
const add = (a, b) => {
// your code here
let total = "";
if (a === 2, b === 1){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a valid condition, you can't use a comma to separate conditions. You need to use logical operators && or ||

};

const round = a => {
// your code here
let total = "";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return Math.round(a)

use the parameters, don't check for specific values!

@@ -1,45 +1,203 @@
const add = (a, b) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module is wrong, almost in all these cases you wrote unnecessary logical statements. You should think about the generic case

@@ -1,25 +1,98 @@
const sayHello = string => {
// your code here
const sayHello = (string) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem here, you wrote code for each specific value rather than generalizing your solution dependent on the parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants